Skip to content

Conversation

@strogiyotec
Copy link
Contributor

@strogiyotec strogiyotec commented Dec 2, 2025

JAVA-4320
1. increase the number of operations to 1000 as in NodeJs
2. increase the deviation to 15% as in NodeJs

Increased local threshold to 3 seconds

@strogiyotec strogiyotec requested a review from a team as a code owner December 2, 2025 23:13
@strogiyotec strogiyotec requested a review from nhachicha December 2, 2025 23:13
@stIncMale
Copy link
Member

This PR suggests we should change the corresponding specification prose test https://github.com/mongodb/specifications/blob/master/source/server-selection/server-selection-tests.md#prose-test. I don't think Node should have just silently deviated from the spec.

@vbabanin vbabanin self-requested a review December 4, 2025 02:52
@strogiyotec
Copy link
Contributor Author

This PR suggests we should change the corresponding specification prose test https://github.com/mongodb/specifications/blob/master/source/server-selection/server-selection-tests.md#prose-test. I don't think Node should have just silently deviated from the spec.

@stIncMale thanks for the feedback, I noticed that dotnet driver doesn't experience flakiness with 10% deviation, the only difference I saw is the threshold value

  1. dotnet uses 1seconds
  2. specification mentioned localThresholdMS=30000

So I updated the PR to have 10% deviation and increased threshold
I also created a patch for evergreen that ran this test 300 times , no failed tests were observed

MongoClientSettings clientSettings = getMongoClientSettingsBuilder()
.applicationName(appName)
.applyConnectionString(multiMongosConnectionString)
// set it to 3000 ms according to specification
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nit] The comment can give the impression that only this setting is spec-compliant, while the rest might not be. Should we remove this one?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of removing the comment, let's update it to explain the full story:

  • our default localThreshold value is 15 ms, which deviates from the default value 3000 ms required by the specification;
  • such a small localThreshold disrupts server selection in this probabilistic test enough to make it fail noticeably often.

Copy link
Member

@stIncMale stIncMale left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants